-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[Clang] Fix handling of immediate escalation for inherited constructors #112860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) ChangesFixes #112677 Full diff: https://github.com/llvm/llvm-project/pull/112860.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a65bd6f382901b..8846ee59a6e241 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -418,7 +418,7 @@ Improvements to Clang's diagnostics
- The warning for an unsupported type for a named register variable is now phrased ``unsupported type for named register variable``,
instead of ``bad type for named register variable``. This makes it clear that the type is not supported at all, rather than being
suboptimal in some way the error fails to mention (#GH111550).
-
+
- Clang now emits a ``-Wdepredcated-literal-operator`` diagnostic, even if the
name was a reserved name, which we improperly allowed to suppress the
diagnostic.
@@ -537,6 +537,7 @@ Bug Fixes to C++ Support
certain situations. (#GH47400), (#GH90896)
- Fix erroneous templated array size calculation leading to crashes in generated code. (#GH41441)
- During the lookup for a base class name, non-type names are ignored. (#GH16855)
+- Fix immediate escalation not propagating through inherited constructors. (#GH112677)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 8321cee0e0bc94..5314d01ed4fb6c 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3284,6 +3284,13 @@ bool FunctionDecl::isImmediateEscalating() const {
// consteval specifier,
if (isDefaulted() && !isConsteval())
return true;
+
+ if (auto *CD = dyn_cast<CXXConstructorDecl>(this);
+ CD && CD->isInheritingConstructor())
+ return CD->getInheritedConstructor()
+ .getConstructor()
+ ->isImmediateEscalating();
+
// - a function that results from the instantiation of a templated entity
// defined with the constexpr specifier.
TemplatedKind TK = getTemplatedKind();
@@ -3304,6 +3311,12 @@ bool FunctionDecl::isImmediateFunction() const {
if (isImmediateEscalating() && BodyContainsImmediateEscalatingExpressions())
return true;
+ if (auto *CD = dyn_cast<CXXConstructorDecl>(this);
+ CD && CD->isInheritingConstructor())
+ return CD->getInheritedConstructor()
+ .getConstructor()
+ ->isImmediateFunction();
+
if (const auto *MD = dyn_cast<CXXMethodDecl>(this);
MD && MD->isLambdaStaticInvoker())
return MD->getParent()->getLambdaCallOperator()->isImmediateFunction();
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index 378414f1361729..187a4958a2ee62 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -496,3 +496,24 @@ struct Y {
template void g<Y>();
}
+
+namespace GH112677 {
+
+class ConstEval {
+ public:
+ consteval ConstEval(int); // expected-note {{declared here}}
+};
+
+struct B {
+ ConstEval val;
+ template <class Anything = int> constexpr
+ B(int arg) : val(arg) {} // expected-note {{undefined constructor 'ConstEval'}}
+};
+struct C : B {
+ using B::B; // expected-note {{in call to 'B<int>(0)'}}
+};
+
+C c(0); // expected-note{{in implicit initialization for inherited constructor of 'C'}}
+// expected-error@-1 {{call to immediate function 'GH112677::C::B' is not a constant expression}}
+
+}
|
|
Consider the following testcase: With this patch, the first one produces an error, the second doesn't. Which... seems dubious? Not sure. Inheriting the "immediate-escalating" property seems surprising to me. |
I am far from a standards expert, but I would expect both to compile? When calling an inherited constructor, "all other bases and members of Derived are initialized as if by the defaulted default constructor", and a "a defaulted special member function that is not declared with the consteval specifier" is an immediate-escalating function. So, my thought is that there is a compiler-generated constructor that is immediate-escalating, which in turn calls the base class constructor, which may or may not be immediate or immediate-escalating. So, in the reduced test case from the original bug #112677, the FakeOptionalBase constructor is immediate-escalating, as is the compiler generated FakeOptional constructor. Since the ConstEval constructor is immediate, the FakeOptionalBase constructor escalates to being immediate, which in turn causes the FakeOptional constructor to escalate to being immediate. In the testcase here, the TemplateCtor constructor is immedate-escalating and SimpleCtor constructor is not, but in neither case is the resulting base constructor immediate. However, I would expect the generated constructor for both ConstEvalMember1 and CostEvalMember2 to be immediate-escalating, and both to then escalate to being immediate due to the call to the immediate ConstEval constructor, as I believe default member initializers used to initialize a member subobject have been clarified to be part of the body of the constructor. |
|
I tried to poke around at this. I believe the simplest change to the PR to match my intuition would be to change if (const auto *CD = dyn_cast<CXXConstructorDecl>(this);
CD && CD->isInheritingConstructor())
return CD->getInheritedConstructor()
.getConstructor()
->isImmediateEscalating();to if (const auto *CD = dyn_cast<CXXConstructorDecl>(this);
CD && CD->isInheritingConstructor())
return true;in However, it might be additionally be desirable to have if (const auto *CD = dyn_cast<CXXConstructorDecl>(this);
CD && CD->isInheritingConstructor())
return CD->getInheritedConstructor()
.getConstructor()
->isImmediateFunction();from if (isImmediateEscalating() && BodyContainsImmediateEscalatingExpressions())
return true;to handle the immediate-inherited-constructor case. However, I am not familiar enough with the code to know the proper way to make that happen. |
f36dc6a to
ca10f9d
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ca10f9d to
620a853
Compare
f8bc77e to
b10d359
Compare
b10d359 to
ecc6e17
Compare
|
@erichkeane mind re-reviewing that? |
erichkeane
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no concerns.
Fixes #112677